Skip to content

Conversation

@karstenkoenig
Copy link
Contributor

Introduce etr-buffer into the tddconf binding to support flexible placement of the etr buffer in the memory map. The location will then be stored into the user information configuration registers and loaded on power on

Introduce etrbuffer in the tddconf bindings to support flexible
placement in the memory map.

Signed-off-by: Karsten Koenig <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the buffer location is flexible, we should move it to cpuapp_ram0x_region instead of maintaining this disjoint region, re: #72273 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did that initially but that means that either cpurad_ram0x_region needs to move down by 4K or cpuapp_data in cpuapp_ram0x_region must be reduced by 4K and figured both aren't that great.
We are going to have a coalescing PR for memory regions anyway after dropping GenB support, maybe it could be done there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not meaning to kick the ball just further to the next person, I can start having a look at the memory map given recent priority changes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The snippet should shrink the cpuapp's SRAM in cpu_app_ram0x_region and place etr_buf_ram0x in there

@karstenkoenig karstenkoenig force-pushed the tddconf_flexible_etrbuffer_upstream branch from 5885efd to 9d06a3a Compare November 15, 2024 09:40
@karstenkoenig
Copy link
Contributor Author

Dropped moving the actual buffer from this PR, that also means this change can apply cleanly without a new sdsc bundle. We can move the memory region when new nrf-regtool and sdsc bundle are released

@karstenkoenig
Copy link
Contributor Author

@hubertmis @57300 can you please check again? I removed the memory section handling and will leave that for when nrf-regtool 8.1 and the associated sdfw is release that actually supports this. It's a bit of a circular dependency and this is the only way to break it up as far as I can tell.

@karstenkoenig
Copy link
Contributor Author

@anangl @masz-nordic Could you please have a look as well? I'd like to get that in so that the sdfw can start relying on the field and then improve the actual memory map

@hubertmis
Copy link
Member

@hubertmis @57300 can you please check again?

What would you like me to check? I hope I'm not blocking this PR. I just answered one question. I'm OK to move the implementation of the required changes to another PR

@karstenkoenig
Copy link
Contributor Author

Sorry nah I think this is good enough, I just tried pushing it forward for the needed number of aprovers to get it merged. The etr memory overlay change will come when sdfw supports it, which is dependent on this here, so this needs to be first

@fabiobaltieri fabiobaltieri merged commit a4fcd5e into zephyrproject-rtos:main Nov 26, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants